Skip to content

Products Onboarding: Enable product previews for all stores #8058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Nov 9, 2022
Merged

Products Onboarding: Enable product previews for all stores #8058

merged 3 commits into from
Nov 9, 2022
+36 −52

Conversation

@rachelmcr
Copy link
Contributor

@rachelmcr rachelmcr commented Nov 8, 2022 •
edited
Loading

Closes: #8046

Description

This PR enables product preview for all stores (instead of only WordPress.com-hosted stores) by using a frame-nonce parameter in the preview URL. The same preview approach is used for all stores, to reduce complexity.

It also updates the Preview button so it's always visible on products that can be previewed (new and draft products), and just disabled when the product is a new blank product. This is better for accessibility, so the button doesn't appear/disappear from the navigation bar.

Caveat: I can't come up with a good way to distinguish template products from new blank products, so the preview button is only enabled once you make changes to the template product details. (This is the same behavior as before.)

Changes

  • Removes the authentication step from displayProductPreview() and just adds the required query parameters (preview and frame-nonce) to the preview URL.
  • Removes the check for new blank products when action buttons are added to the navigation bar, and moves that to a separate check shouldEnablePreviewButton() to determine if the Preview button should be enabled.

Testing

  1. Build and run the app in debug or alpha mode.
  2. Select a store not hosted on WordPress.com.
  3. Go to the Products tab and add a new product. Confirm the Preview button is visible but disabled, and it is enabled once you start making changes.
  4. Tap the Preview button. Confirm the new product is saved and you can see a preview on your store.

Screenshots

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2022-11-08.at.17.51.19.mp4

Submitter Checklist

Update release notes:

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@rachelmcr rachelmcr added status: feature-flagged Behind a feature flag. Milestone is not strongly held. feature: products onboarding Related to onboarding new users to manage products labels Nov 8, 2022
@rachelmcr rachelmcr added this to the 11.2 milestone Nov 8, 2022
@peril-woocommerce
Copy link

peril-woocommerce bot commented Nov 8, 2022

Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 2 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

@rachelmcr rachelmcr requested a review from Ecarrion November 8, 2022 18:34
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 8, 2022 •
edited
Loading

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr8058-d947b78 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@ealeksandrov ealeksandrov self-assigned this Nov 9, 2022
ealeksandrov
ealeksandrov approved these changes Nov 9, 2022
View reviewed changes
Copy link
Contributor

@ealeksandrov ealeksandrov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All test cases work well and look good for me 🚀
✅ self-hosted site
✅ wp.com site
✅ wp.com site with mapped domain

WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewModelProtocol.swift
///
func shouldEnablePreviewButton() -> Bool {
!(formType == .add && !hasUnsavedChanges())
}
Copy link
Contributor

@ealeksandrov ealeksandrov Nov 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea about toggling the button state instead of hiding it!

Copy link
Contributor

@Ecarrion Ecarrion Nov 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to change this condition due to the template experiment, as a new template product would be considered as a new product without any changes.

I think we need to add a check for the "blank" state.

Copy link
Contributor Author

@rachelmcr rachelmcr Nov 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to add a check for the "blank" state.

I agree, but I haven't come up with a good solution for it yet. This PR doesn't change whether you can preview new template products (it just makes the preview button visible but disabled, instead of not appearing at all), so I'm going to go ahead and merge it. I've opened a new issue #8071 so we can follow up with a fix for new template products.

@ealeksandrov ealeksandrov removed their assignment Nov 9, 2022
@rachelmcr rachelmcr mentioned this pull request Nov 9, 2022
Closed
@rachelmcr rachelmcr merged commit ea5de4a into trunk Nov 9, 2022
@rachelmcr rachelmcr deleted the issue/8046-preview-with-frame-nonce branch November 9, 2022 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ecarrion Ecarrion Awaiting requested review from Ecarrion

1 more reviewer

@ealeksandrov ealeksandrov ealeksandrov approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

feature: products onboarding Related to onboarding new users to manage products status: feature-flagged Behind a feature flag. Milestone is not strongly held.

Projects

None yet

Milestone

11.2

Development

Successfully merging this pull request may close these issues.

Product Preview: WebView Authentication (self-hosted sites)

5 participants

@rachelmcr @wpmobilebot @Ecarrion @ealeksandrov